runtime: minor bugs fixed in coreclr and libraries #97155
Conversation
…cpp:262, may be NULL and is dereferenced at filetime.cpp:268>. The error was detected using the Svace analyzer maded by ISP RAS.
…d at process.cpp:2824 without checking for NULL, but it is usually checked for this function (11/12)>. The error was detected using the Svace analyzer maded by ISP RAS.
…on with possible null return value, is dereferenced in member access expression sectionRecord.HasLocationInput>. The error was detected using the Svace analyzer maded by ISP RAS.
…h AS-cast should be compared with null instead of method>. The error was detected using the Svace analyzer maded by ISP RAS.
ghost
added
the
community-contribution
dotnet-issue-labeler
bot
added
the
needs-area-label
|
@dotnet-policy-service agree |
| @@ -335,7 +335,7 @@ internal ConfigurationSection FindImmediateParentSection(ConfigurationSection se | |||
|
|
|||
| string configKey = section.SectionInformation.SectionName; | |||
| SectionRecord sectionRecord = GetSectionRecord(configKey, false); | |||
| if (sectionRecord.HasLocationInputs) | |||
| if ((sectionRecord != null) && sectionRecord.HasLocationInputs) | |||
There was a problem hiding this comment.
This doesn't actually fix a null reference exception. If sectionRecord could actually be null, this will cause the else block to be executed, which also immediately dereferences sectionRecord.
Unless there's a known repro for this causing a problem, let's revert this.
There was a problem hiding this comment.
Stefan, to correct the error, I propose below in line 346 to change the condition to <if ((sectionRecord != null) && sectionRecord.HasIndirectLocationInputs)>. Stefan, judging by the results of the static analysis, the value returned by the function GetSectionRecord(...) in the library System.Configuration.ConfigurationManager code base should be checked for NULL (you can search (sectionRecord==null or sectionRecord!=null) by searching in the files MgmtConfigurationRecord.cs and BaseConfigurationRecord.cs). I suggest you fix it. It is not possible to check for an error in this case using a test, because it was detected using a static analysis tool.
There was a problem hiding this comment.
This method is searching for immediate parent section. I assume that the immediate parent section is expected to always exist and that's why there is no null check. Maybe we can add Debug.Assert instead?
There was a problem hiding this comment.
Yes, let's try, just explain what needs to be done for this.
| @@ -263,6 +263,7 @@ BOOL PALAPI FileTimeToSystemTime( CONST FILETIME * lpFileTime, | |||
| #else /* HAVE_GMTIME_R */ | |||
| UnixSystemTime = gmtime( &UnixFileTime ); | |||
| #endif /* HAVE_GMTIME_R */ | |||
| if (!UnixSystemTime) return FALSE; | |||
There was a problem hiding this comment.
| if (!UnixSystemTime) return FALSE; | |
| if (!UnixSystemTime) | |
| { | |
| ERROR( "gmtime failed.\n" ); | |
| SetLastError(ERROR_INVALID_PARAMETER); | |
| return FALSE; | |
| } |
There was a problem hiding this comment.
Jan, thanks for the correction, we'll take it into account for the future.
...ystem.Configuration.ConfigurationManager/src/System/Configuration/MgmtConfigurationRecord.cs
Outdated
Show resolved
Hide resolved
...ystem.Configuration.ConfigurationManager/src/System/Configuration/MgmtConfigurationRecord.cs
Outdated
Show resolved
Hide resolved
…stem/Configuration/MgmtConfigurationRecord.cs Co-authored-by: Jan Kotas <jkotas@microsoft.com>
…stem/Configuration/MgmtConfigurationRecord.cs Co-authored-by: Jan Kotas <jkotas@microsoft.com>
|
Thanks for the work you've done. |
* Fixed error: <Pointer, returned from function 'gmtime_r' at filetime.cpp:262, may be NULL and is dereferenced at filetime.cpp:268>. The error was detected using the Svace analyzer maded by ISP RAS. * Fixed error: <Return value of a function 'PAL_wcsrchr' is dereferenced at process.cpp:2824 without checking for NULL, but it is usually checked for this function (11/12)>. The error was detected using the Svace analyzer maded by ISP RAS. * Fixed error: <Value sectionRecord, which is result of method invocation with possible null return value, is dereferenced in member access expression sectionRecord.HasLocationInput>. The error was detected using the Svace analyzer maded by ISP RAS. * Fixed error: <Maybe ecmaCandidateMethod, that created from method with AS-cast should be compared with null instead of method>. The error was detected using the Svace analyzer maded by ISP RAS. * Update filetime.cpp * Update process.cpp * Update MgmtConfigurationRecord.cs * Update src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/MgmtConfigurationRecord.cs Co-authored-by: Jan Kotas <jkotas@microsoft.com> * Update src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/MgmtConfigurationRecord.cs Co-authored-by: Jan Kotas <jkotas@microsoft.com> --------- Co-authored-by: Jan Kotas <jkotas@microsoft.com>
The bugs fixed are related to:
-situations where a pointer is dereferenced, but can only have NULL value, and so the operation of dereference can never be run without causing a runtime error;
-after some value is retrieved by as operator, the original value is checked for null instead of the new one.
Found by Linux Verification Center (linuxtesting.org) with SVACE.